Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update page titles for dashboards and alerts #6706

Merged
merged 4 commits into from
Jan 17, 2025

Conversation

nishantharcot
Copy link
Contributor

@nishantharcot nishantharcot commented Dec 24, 2024

Summary

The titles of dashboards and alerts are updated to their names.

Screenshots

Alert-Title Dashboard-Title

Important

Update document.title in AlertDetails.tsx and DashboardPage.tsx to reflect alert and dashboard names.

  • Behavior:
    • Update document.title in AlertDetails.tsx to the alert name from alertDetailsResponse.
    • Update document.title in DashboardPage.tsx to the dashboard name from dashboardResponse.
  • Files Affected:
    • AlertDetails.tsx
    • DashboardPage.tsx

This description was created by Ellipsis for 86a74a6. It will automatically update as commits are pushed.

Copy link

welcome bot commented Dec 24, 2024

Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗

@CLAassistant
Copy link

CLAassistant commented Dec 24, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 86a74a6 in 36 seconds

More details
  • Looked at 29 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/pages/AlertDetails/AlertDetails.tsx:84
  • Draft comment:
    Consider extracting the document title update logic into a reusable function to adhere to the DRY principle. This logic is also present in DashboardPage.tsx.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The document title update logic is repeated in both files. It would be better to extract this logic into a reusable function to adhere to the DRY principle.
2. frontend/src/pages/NewDashboard/DashboardPage.tsx:21
  • Draft comment:
    Avoid using inline styles for setting document titles. Consider using a React effect hook to manage side effects like updating the document title.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_UBLf4YItn1uZTHvi


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

frontend/src/pages/AlertDetails/AlertDetails.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@SagarRajput-7 SagarRajput-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@srikanthccv srikanthccv enabled auto-merge (squash) January 17, 2025 08:54
@srikanthccv srikanthccv merged commit c66cd3c into SigNoz:main Jan 17, 2025
15 of 16 checks passed
Copy link

welcome bot commented Jan 17, 2025

Congrats on merging your first pull request!
minion-party
We here at SigNoz are proud of you! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants